refactor: Centralize error output #3902

arng40 · 2025-11-05T09:53:01Z

Remove code duplication found in GEOS_THROW, GEOS_ERROR, GEOS_WARNING and put into a static function in ErrorLogger.
Called while flushErrorMsg().
Move all Exceptions under GeosExceptions.hpp

…file

…necessary

… link between GEOS_THROW_CTX_IF and LVARRAY_THROW_IF_TEST( EXP, MSG, TYPE )

… in try/catch statements Problem: Retrieves everything that was thrown, so not just the message.

…/catch in main)": remove useless try/catch

…y spaces. The previous condition checked whether an argument was present and whether the option was immediately followed by a value like -test"value", which excluded valid cases like -test "value" et -test "value".

…t is expected.

src/coreComponents/common/logger/ErrorHandling.hpp

src/coreComponents/common/logger/ErrorHandling.cpp

src/coreComponents/common/logger/ErrorHandling.hpp

src/coreComponents/common/logger/ErrorHandling.cpp

src/coreComponents/common/logger/ErrorHandling.hpp

MelReyCG · 2025-12-17T11:16:27Z

src/main/main.cpp

+  }
+  catch( std::exception const & e )
+  { // native exceptions management
+    ErrorLogger::ErrorMsg & errMsg = ErrorLogger::global().currentErrorMsg();


I think it lacks a lot of information here: at this point, IIRC, GEOS_THOW() has never been called, so the same information must be provided to the error message.

Appart from that, this part is a lot better!

…larity

src/coreComponents/common/logger/ErrorHandling.cpp

src/coreComponents/common/logger/ErrorHandling.hpp

MelReyCG · 2025-12-29T14:45:50Z

src/coreComponents/common/logger/ErrorHandling.hpp

-   *        potencially at various application layers
-   *        Use flushErrorMsg() when the message is fully constructed and you want it to be output
-   * @return Reference to the current instance for method chaining.
+   *        potencially at various application layers (Typically for exceptions)


"Typically for exceptions"
Are there other use-cases?

We have now 2 flush methods

src/coreComponents/common/logger/ErrorHandling.hpp

src/coreComponents/dataRepository/unitTests/testErrorHandling.cpp

src/coreComponents/common/logger/ErrorHandling.hpp

MelReyCG · 2025-12-29T16:33:36Z

src/coreComponents/common/logger/ErrorHandling.hpp

+    ErrorContext( map< Attribute, std::string >  attributes, integer priority ):
+      m_dataDisplayString( "" ),
+      m_attributes( attributes ),
+      m_priority( priority ) {};


m_dataDisplayString is allowed as empty? Isn't that the source of your issue in the unit test?

Here is what is needed : ErrorContext( string formattedContext, map< Attribute, std::string > attributes, integer priority ):

MelReyCG · 2025-12-29T16:35:14Z

src/coreComponents/common/initializeEnvironment.cpp

+      ErrorLogger::global().beginLogger()
+        .addSignalToMsg( signal )
+        .setType( ErrorLogger::MsgType::Error )
+        .addRank( ::geos::logger::internal::g_rank )
+        .addCallStackInfo( stackHistory )
+        .addContextInfo(
+        ErrorContext{ { { ErrorContext::Attribute::Signal, std::to_string( signal ) } }, 1 },
+        ErrorContext{ { { ErrorContext::Attribute::DetectionLoc, string( "signal handler" ) } }, 0 } )
+        .flush();


Maybe a unit test for signals would be a good thing.

src/coreComponents/common/logger/ErrorHandling.hpp

MelReyCG · 2026-01-08T16:04:18Z

src/coreComponents/dataRepository/unitTests/testErrorHandling.cpp

+      "***** - {}\n",
+      testErrorLogger.getCurrentExceptionMsg().m_file, line1,
+      testErrorLogger.getCurrentExceptionMsg().m_cause,
+      context.toString(),


Shouldn't you switch with the most important context here?

MelReyCG · 2026-01-08T16:14:30Z

src/coreComponents/common/logger/GeosExceptions.hpp

+  /// Error message logger for structured error reporting
+  DiagnosticMsg m_errorMsg;


isn't that in currentExceptionMsg()?

MelReyCG · 2026-01-08T16:19:18Z

src/coreComponents/common/logger/GeosExceptions.hpp

+  void prepareWhat( std::string const & msg,
+                    std::string const & cause,
+                    char const * file,
+                    int line,
+                    int rank,
+                    string_view stackTrace ) noexcept
+  {
+    try
+    {
+      std::ostringstream oss;
+      oss << "***** GEOS Exception\n";
+      oss << "***** LOCATION: " << file << ":" << line << "\n";
+      oss << "***** " << cause << "\n";
+      oss << "***** Rank  " << rank << ": "<< msg <<"\n\n";
+      oss << stackTrace;
+      m_cachedWhat = oss.str();
+    } catch( ... )
+    {
+      m_cachedWhat = "GEOS Exception (formatting failed)";
+    }
+
+  }


Suggested change

void prepareWhat( std::string const & msg,

std::string const & cause,

char const * file,

int line,

int rank,

string_view stackTrace ) noexcept

{

try

{

std::ostringstream oss;

oss << "***** GEOS Exception\n";

oss << "***** LOCATION: " << file << ":" << line << "\n";

oss << "***** " << cause << "\n";

oss << "***** Rank " << rank << ": "<< msg <<"\n\n";

oss << stackTrace;

m_cachedWhat = oss.str();

} catch( ... )

{

m_cachedWhat = "GEOS Exception (formatting failed)";

}

}

private:

static thread_local std::ostringstream formattingOSS;

public:

void prepareWhat( ErrorMsg const & msg ) noexcept

{

formattingOSS.str("");

formattingOSS.clear();

ErrorLogger::writeToAscii( msg, formattingOSS );

m_cachedWhat = formattingOSS.bad ? "Exception formatting error!" : formattingOSS.str();

m_errorMsg = msg;

}

MelReyCG · 2026-01-08T16:20:22Z

src/coreComponents/dataRepository/unitTests/testErrorHandling.cpp

+      .addToMsg( e.what() )
+      .addRank( ::geos::logger::internal::g_rank )
+      .addCallStackInfo( LvArray::system::stackTrace( true ) )
+      .get();


who get what?

MelReyCG · 2026-01-08T16:34:15Z

src/coreComponents/common/logger/ErrorHandling.hpp

+  /// The diagnosticMsg being constructed
+  DiagnosticMsg m_errorMsg;
+  /// The target diagnosticMsg
+  DiagnosticMsg * m_targetErrorMsg = nullptr;


Suggested change

/// The diagnosticMsg being constructed

DiagnosticMsg m_errorMsg;

/// The target diagnosticMsg

DiagnosticMsg * m_targetErrorMsg = nullptr;

/// The target diagnosticMsg

DiagnosticMsg & m_targetErrorMsg;

MelReyCG · 2026-01-08T16:47:52Z

src/coreComponents/common/logger/ErrorHandling.hpp

+    ErrorContext( map< Attribute, std::string >  attributes, integer priority ):
+      m_dataDisplayString( "" ),
+      m_attributes( attributes ),
+      m_priority( priority ) {};


Here is what is needed : ErrorContext( string formattedContext, map< Attribute, std::string > attributes, integer priority ):

MelReyCG · 2026-01-08T16:52:46Z

src/coreComponents/common/logger/ErrorHandling.hpp

+
+  /**
+   * @brief Write all the information retrieved about the error/warning message into the YAML file
+   * @param errorMsg a constant reference to the error


MelReyCG · 2026-01-08T16:54:09Z

src/coreComponents/common/logger/ErrorHandling.hpp

+  void flushCurrentExceptionMessage();
+
+  /**
+   * @brief Write all the information retrieved about the diagnostic message into the instance
+   * outputs (stream specified + optional yaml file)
+   * @param errorMsg a reference to the ErrorMsg to output, and will be re-initialized
+   * @note Used for warnings and non-exception errors
+   */
+  void flushErrorMsg( DiagnosticMsg & errMsg );


provide a oss parameter (with a default to std::cout)?

src/coreComponents/common/logger/ErrorHandling.hpp

MelReyCG · 2026-01-08T17:01:47Z

src/coreComponents/common/logger/ErrorHandling.hpp

+  static DiagnosticMsgBuilder init()
+  { return DiagnosticMsgBuilder();}


static ErrorMsgBuilder init( ErrorMsg & msg, MsgType msgType, std::string_view msgContent, integer rank, std::string_view msgFile, integer msgLine );

What do you think of this suggestion? ^
Is that too constraining?

(internally, that would call the builder methods)

ok for init with parameters but I removed file & line. For the signal we don't have file & line

amandinehry added 30 commits April 25, 2025 17:05

creation of an object header that logs errors and warnings in a yaml …

c846586

…file

Set up ErrorHandling

f1713fa

traits errors commented (to treat)

4e5250f

Set up test

94b775c

Modification to properly format the yaml so that it can be parsed if …

1cb8aa3

…necessary

Test to add variadic arguments

9d5e16a

draft: retrieve data contexts info

2f7660c

Added additional information in the yaml - not functional missing the…

6036551

… link between GEOS_THROW_CTX_IF and LVARRAY_THROW_IF_TEST( EXP, MSG, TYPE )

First attempt at outputting exceptions

46b4904

Complete: code for outputting errors into yaml

e1aed93

Manage the text contained in the potential exception

bc953be

First attempt: Handling the text contained in the potential exception…

1a434bc

… in try/catch statements Problem: Retrieves everything that was thrown, so not just the message.

Complete: yaml output for PVT tables error

acf4d99

YAML error message well formatted

ee58884

Fix duplication error

32a0237

Add variadic parameters to GEOS_THROW_CTX_IF

589270f

Reorganization after operational tests

9b65581

Apply the output error functionality in yaml to all geos errors

b13f0a6

Latest update for exception handling (try/catch in main)

6de2c6a

Add priority to the error message into the yaml file

5830aee

Update of the commit named "latest update for exception handling (try…

2b27c41

…/catch in main)": remove useless try/catch

set priority

86f793e

errors cli modification: add --errors-output

8a8117e

errors cli modification: add --errors-output

2c0f5aa

Replace original macros

eb2f691

Correction: replace original macros

7b5f293

Reacting with the command line option -e / --errors-output

670ddfa

Add the rank on which the error is catch for error and warning outputs

d08a30f

Unit test to check if the contents of the yaml file match exactly wha…

5a6c1a0

…t is expected.

arng40 and others added 4 commits December 10, 2025 17:56

fix test

daec006

Uncrustify

ab9895b

Merge branch 'develop' into refactor/dudes/error-context

60a1f3a

Merge branch 'develop' into refactor/dudes/error-context

4e74b00

MelReyCG requested changes Dec 17, 2025

View reviewed changes

arng40 and others added 6 commits December 19, 2025 15:42

reorder function + add constructor to ErrorContext

08973a3

uncrustify & doxygen

cee0059

Merge branch 'develop' into refactor/dudes/error-context

2a639a7

Merge branch 'develop' into refactor/dudes/error-context

a2b31d1

improve writeToAscii

d4db78b

add context to std::exception

7df78ff

arng40 mentioned this pull request Dec 23, 2025

feat: Error reporting at the end of the simulatiuon #3929

Draft

arng40 added 4 commits December 24, 2025 15:33

fix encapsulation in ErrorLogger by adding BuilderPattern + improve c…

abc6d2a

…larity

uncrustify

ec7ddc1

remove redundant lmsg in case of multiple rank

4a5b4fe

missing const

35d8707

MelReyCG requested changes Dec 29, 2025

View reviewed changes

MelReyCG reviewed Jan 5, 2026

View reviewed changes

src/coreComponents/common/logger/ErrorHandling.hpp Outdated Show resolved Hide resolved

src/coreComponents/common/logger/ErrorHandling.hpp Outdated Show resolved Hide resolved

arng40 added 5 commits January 5, 2026 14:22

delete ErrorMsg method & remove unecessary method in Builder

38023c2

extract inerclass & refacto ErrorBuilder

62fcf31

rename ErrorMsg & adapt errorLogger function called in catch

1495a68

modify yaml & renaming & fix

a4e69cd

remove debug log

16057c1

MelReyCG requested changes Jan 8, 2026

View reviewed changes

arng40 added 5 commits January 9, 2026 14:35

update prepareWhat

3ed52be

update builder

e40167f

rename get method & sort errorContext

b529d7a

remove debug

ec4703c

update format

fbed3ed

		/// Error message logger for structured error reporting
		DiagnosticMsg m_errorMsg;

		static DiagnosticMsgBuilder init()
		{ return DiagnosticMsgBuilder();}

refactor: Centralize error output #3902

Are you sure you want to change the base?

refactor: Centralize error output #3902

Conversation

arng40 commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MelReyCG Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

arng40 commented Nov 5, 2025 •

edited

Loading

MelReyCG Jan 8, 2026 •

edited

Loading